Skip to content

Feature/support keypair generation#2815

Open
HunsupJung wants to merge 1 commit intomainfrom
feature/support-keypair-generation
Open

Feature/support keypair generation#2815
HunsupJung wants to merge 1 commit intomainfrom
feature/support-keypair-generation

Conversation

@HunsupJung
Copy link
Collaborator

@HunsupJung HunsupJung commented Feb 27, 2026 •
edited
Loading

Type of Change

  • WWST Certification Request
    • If this is your first time contributing code:
      • I have reviewed the README.md file
      • I have reviewed the CODE_OF_CONDUCT.md file
      • I have signed the CLA
    • I plan on entering a WWST Certification Request or have entered a request through the WWST Certification console at developer.smartthings.com
  • Bug fix
  • New feature
  • Refactor

Checklist

  • I have performed a self-review of my code
  • I have commented my code in hard-to-understand areas
  • I have verified my changes by testing with a device or have communicated a plan for testing
  • I am adding new behavior, such as adding a sub-driver, and have added and run new unit tests to cover the new behavior

Description of Change

Summary of Completed Tests

@github-actions
Copy link

github-actions bot commented Feb 27, 2026

Invitation URL:
https://bestow-regional.api.smartthings.com/invite/adMKXL5K4J29

@github-actions
Copy link

github-actions bot commented Feb 27, 2026 •
edited
Loading

Test Results

   72 files    493 suites   0s ⏱️
2 700 tests 2 700 ✅ 0 💤 0 ❌
4 562 runs  4 562 ✅ 0 💤 0 ❌

Results for commit 98a69db.

♻️ This comment has been updated with latest results.

@HunsupJung
Copy link
Collaborator Author

HunsupJung commented Feb 27, 2026

@tpmanley
I entered the Git command incorrectly and the PR was closed, but I couldn't open it again to see if I didn't have the authority, so I made a new one. I'm sorry to bother you.

@github-actions
Copy link

github-actions bot commented Feb 27, 2026 •
edited
Loading

File Coverage
All files 80% ❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-lock/src/new-matter-lock/init.lua 75% ❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-lock/src/new-matter-lock/can_handle.lua 90% ✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-lock/src/lazy_load_subdriver.lua 57% ❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-lock/src/init.lua 92% ✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-lock/src/lock_utils.lua 98% ✅

Minimum allowed coverage is 90%

Generated by 🐒 cobertura-action against 98a69db

@HunsupJung
Copy link
Collaborator Author

HunsupJung commented Feb 27, 2026

@tpmanley

Thanks for the making the requested changes @HunsupJung . The driver code is looking pretty good to me now. In addition to what @ctowns suggested, can you also take a look at the unit tests failures reported by CI and try to add tests for the new code?

First of all, I corrected all the errors.

@HunsupJung
Copy link
Collaborator Author

HunsupJung commented Feb 27, 2026

@ctowns
if test code uses lock-modular, the following message appears as if lockAliro capability is not supported, making it impossible to write test code. What do you suggest I do?

INFO  || <MatterDevice: 00000000-1111-2222-3333-000000000001 [nil]> received InteractionResponse: <InteractionResponse || type: REPORT_DATA, response_blocks: [<InteractionResponseInfoBlock || status: SUCCESS, <InteractionInfoBlock || endpoint: 0x01, cluster: DoorLock, attribute: AliroReaderVerificationKey, data: OctetString1: "\x04\xA9\xCB\xE4\x18\xEB\x09\x66\x16\x43\xE2\xA4\xA8\x46\xB8\xED\xFE\x27\x86\x98\x30\x2E\x9F\xB4\x3E\x9B\xFF\xD3\xE3\x10\xCC\x2C\x2C\x7F\xF4\x02\xE0\x6E\x40\xEA\x3C\xE1\x29\x43\x52\x73\x36\x68\x3F\xC5\xB1\xCB\x0C\x6A\x7C\x3F\x0B\x5A\xFF\x78\x35\xDF\x21\xC6\x24">>]>
TRACE || Found MatterMessageDispatcher handler in matter-lock -> New Matter Lock Handler
WARN  || Attempted to generate event for 00000000-1111-2222-3333-000000000001.main but it does not support capability Lock Aliro

@hcarter-775
Copy link
Contributor

hcarter-775 commented Feb 27, 2026

@HunsupJung when you're setting up the test, you can make a mock device with the following profile
profile = t_utils.get_profile_definition("lock-modular.yml", {enabled_optional_capabilities = {{"main", {"lockAliro"}}}}),

Then run the test as normal. This will make the tested profile include the optional capability. You can add as many optional capabilities as you want in this way.

nickolas-deboom
nickolas-deboom reviewed Feb 27, 2026
View reviewed changes
drivers/SmartThings/matter-lock/src/new-matter-lock/init.lua Outdated
hex_string_to_octet_string(groupResolvingKey)
)
)
device:set_field(lock_utils.ALIRO_READER_CONFIG_UPDATED, true)
Copy link
Contributor

@nickolas-deboom nickolas-deboom Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be worth checking the response of SetAliroReaderConfig before setting ALIRO_READER_CONFIG_UPDATED to true, since if the command fails then set_reader_config would never retry.

Copy link
Collaborator Author

@HunsupJung HunsupJung Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no way to whether the ReaderConfig is set well in SetAliroReaderConfig command and response. So, I will move it to aliro_reader_verification_key_handler which is one of the ReaderConfig.

@HunsupJung HunsupJung force-pushed the feature/support-keypair-generation branch from a90c386 to b331d2d Compare March 3, 2026 13:29
@HunsupJung HunsupJung requested a review from tpmanley March 12, 2026 01:40
@HunsupJung
Copy link
Collaborator Author

HunsupJung commented Mar 12, 2026

@tpmanley
I completed update. Could you review it again?

@tpmanley
Copy link
Contributor

tpmanley commented Mar 12, 2026

Thanks for resolving my two comments. There are a couple of conflicts right now so what I'd recommend is squash and rebase your changes into a single commit on top of latest main branch and then we can give it hopefully the last final review.

@HunsupJung
Support keypair generation
98a69db 
Signed-off-by: Hunsup Jung <hunsup.jung@samsung.com>
@HunsupJung HunsupJung force-pushed the feature/support-keypair-generation branch from 199cfbd to 98a69db Compare March 13, 2026 11:15
@HunsupJung
Copy link
Collaborator Author

HunsupJung commented Mar 13, 2026

@tpmanley
I did squash and rebase the commits.

hcarter-775
hcarter-775 reviewed Mar 13, 2026
View reviewed changes
drivers/SmartThings/matter-lock/src/new-matter-lock/init.lua
end
end
end
match_profile(driver, device)
Copy link
Contributor

@hcarter-775 hcarter-775 Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Due to the logic here, we should do ensure something like this is handled: #2750 (comment) before updating profiles, since this permits the possibility that some features may be disabled later, and we do not want to lose capabilities if that happens.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hcarter-775 hcarter-775 hcarter-775 left review comments

@ctowns ctowns Awaiting requested review from ctowns

@Kwang-Hui Kwang-Hui Awaiting requested review from Kwang-Hui

@nickolas-deboom nickolas-deboom Awaiting requested review from nickolas-deboom

@tpmanley tpmanley Awaiting requested review from tpmanley

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@HunsupJung @hcarter-775 @tpmanley @nickolas-deboom